-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UV-5R: fine tune Aux block upload restrictions - related to #10505 #726
Conversation
If this is acceptable, you can merge if you want. Or I can add it to #10505 tomorrow for testing. |
"The 'Other Settings' and 'Service Settings' were skipped " | ||
"because the firmware version of the image (%s) does not " | ||
"match that of the radio (%s).") | ||
raise errors.RadioError(msg % (image_version, radio_version)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to figure out a better way to do this. I need to do some thinking on it, so don't let me forget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. The problem here is that Baofeng pretty much stopped separating the various models by using different magics. So it is entirely possible to upload an image from a dual-band radio into a tri-band radio hosing the band limits. It is a similar situation between radios with 2 TX power levels and 3 TX power levels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I mean is a better way to raise a message to the user after cloning that isn't an error. So that you don't have to say "ERROR: This is not an error" :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I've never liked this. People see the error symbol and never read what it says and always thing something bad happened. You are the one that suggested that I add the "This is not an error" part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I remember. I'll do some thinking.
_ani = self._memobj.ani | ||
_fm_presets = self._memobj.fm_presets | ||
_settings = self._memobj.settings | ||
_squelch = self._memobj.squelch_new | ||
_vfoa = self._memobj.vfoa | ||
_vfob = self._memobj.vfob | ||
_wmchannel = self._memobj.wmchannel | ||
HN5RV = "HN5RV" in str(self._memobj.firmware_msg.line1) | ||
_has_fmradio = (str(_mem.bcstfmlo) == " FM 65-75M" and | ||
str(_mem.bcstfmhi) == " FM 76-108M") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, these strings are in the memory image and not changeable by the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They not changeable by the typical CHIRP user because they are not exposed to the user. But they are not immutable like the firmware version is in Aux memory block, if that's what you mean. They could be changed by someone that really wanted to change them.
It is for this reason that I block the upload of this region of memory if either or both the image or radio does not have these strings so they can't get overwritten by normal CHIRP use.
_skip_aux_block = False | ||
# check source image for invalid bcst FM data | ||
elif "\xFF" in str(radio._memobj.bcstfmlo) or \ | ||
"\xFF" in str(radio._memobj.bcstfmhi): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay and this is the check for upload.
#seekto 0x0F48; // some new radios do not support this feature | ||
char bcstfmlo[14]; // broadcast FM 65-75 MHz band | ||
u16 fm_presets; // broadcast FM frequency | ||
char bcstfmhi[14]; // broadcast FM 75-108 MHz band |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see you added these elements here just now, I got it.
I guess maybe these could be different in some radios for other geographies maybe? I guess we'll just have to see, but if the new radios are the only ones with 0xFF
in them, then we'll catch those anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I only have 1 radio here with the issue. But the 2 or 3 images that I have gotten from others all have bcstfmlo[14] and bcstfmhi[14] fully populated with 0xFF. So far I have never seen an 0xFF in an older "good" radio so I just check for a single 0xFF when uploading. I would rather err on the side of not uploading to a "good" radio than accidentally upload to a "bad" one.
I won't be around this weekend to merge stuff, so let's just go with this and we can revert later if something isn't right. Thanks! |
CHIRP PR Checklist
The following must be true before PRs can be merged:
tests/images
(except for thin aliases where the driver is sufficiently tested already).Please also follow these guidelines:
six
,future
, etc).NEEDS_COMPAT_SERIAL=False
and useMemoryMapBytes
.tox -emakesupported
and include it in your commit.